From cef86c10d4122588aec8e917902ea30f2983a4c9 Mon Sep 17 00:00:00 2001 From: Nicolas Koch Date: Thu, 10 Sep 2015 02:42:04 +0200 Subject: [PATCH] Restructure cargo_test code and introduce CargoTestError --- src/bin/bench.rs | 5 +- src/bin/test.rs | 5 +- src/cargo/ops/cargo_test.rs | 193 +++++++++++++++++++----------------- src/cargo/util/errors.rs | 45 +++++++++ src/cargo/util/mod.rs | 2 +- 5 files changed, 154 insertions(+), 96 deletions(-) diff --git a/src/bin/bench.rs b/src/bin/bench.rs index c90bdba2d..655fbff60 100644 --- a/src/bin/bench.rs +++ b/src/bin/bench.rs @@ -95,7 +95,10 @@ pub fn execute(options: Options, config: &Config) -> CliResult> { match err { None => Ok(None), Some(err) => { - Err(CliError::from_error(Human(err), 101)) + Err(match err.exit.as_ref().and_then(|e| e.code()) { + Some(i) => CliError::new("", i), + None => CliError::from_error(Human(err), 101) + }) } } } diff --git a/src/bin/test.rs b/src/bin/test.rs index 424e8519c..ade460165 100644 --- a/src/bin/test.rs +++ b/src/bin/test.rs @@ -101,7 +101,10 @@ pub fn execute(options: Options, config: &Config) -> CliResult> { match err { None => Ok(None), Some(err) => { - Err(CliError::from_error(Human(err), 101)) + Err(match err.exit.as_ref().and_then(|e| e.code()) { + Some(i) => CliError::new("", i), + None => CliError::from_error(Human(err), 101) + }) } } } diff --git a/src/cargo/ops/cargo_test.rs b/src/cargo/ops/cargo_test.rs index 6c7227cc2..6370f5c02 100644 --- a/src/cargo/ops/cargo_test.rs +++ b/src/cargo/ops/cargo_test.rs @@ -4,7 +4,7 @@ use std::path::Path; use core::Source; use sources::PathSource; use ops::{self, ExecEngine, ProcessEngine, Compilation}; -use util::{self, CargoResult, CargoError, internal}; +use util::{self, CargoResult, CargoTestError, ProcessError}; pub struct TestOptions<'a> { pub compile_opts: ops::CompileOptions<'a>, @@ -12,47 +12,124 @@ pub struct TestOptions<'a> { pub no_fail_fast: bool, } + + #[allow(deprecated)] // connect => join in 1.3 pub fn run_tests(manifest_path: &Path, options: &TestOptions, - test_args: &[String]) -> CargoResult>> { - let config = options.compile_opts.config; - let (compile, error) = try!(build_and_run(manifest_path, options, test_args)); + test_args: &[String]) -> CargoResult> { + let compilation = try!(compile_tests(manifest_path, options)); + + if options.no_run { + return Ok(None) + } + let mut errors = try!(run_unit_tests(options, test_args, &compilation)); // If we have an error and want to fail fast, return - if error.is_some() && !options.no_fail_fast { - return Ok(error) + if errors.len() > 0 && !options.no_fail_fast { + return Ok(Some(CargoTestError::from(&errors[..]))) } + // If a specific test was requested or we're not running any tests at all, // don't run any doc tests. if let ops::CompileFilter::Only { .. } = options.compile_opts.filter { - return Ok(None) + match errors.len() { + 0 => return Ok(None), + _ => return Ok(Some(CargoTestError::from(&errors[..]))) + } } - if options.no_run { - return Ok(None) + + errors.extend(try!(run_doc_tests(options, test_args, &compilation))); + if errors.len() == 0 { + Ok(None) + } else { + Ok(Some(CargoTestError::from(&errors[..]))) + } +} + +pub fn run_benches(manifest_path: &Path, + options: &TestOptions, + args: &[String]) -> CargoResult> { + let mut args = args.to_vec(); + args.push("--bench".to_string()); + let compilation = try!(compile_tests(manifest_path, options)); + let errors = try!(run_unit_tests(options, &args, &compilation)); + match errors.len() { + 0 => Ok(None), + _ => Ok(Some(CargoTestError::from(&errors[..]))), } +} - let libs = compile.package.targets().iter() - .filter(|t| t.doctested()) - .map(|t| (t.src_path(), t.name(), t.crate_name())); +fn compile_tests<'a>(manifest_path: &Path, + options: &TestOptions<'a>) + -> CargoResult> { + let config = options.compile_opts.config; + let mut source = try!(PathSource::for_path(&manifest_path.parent().unwrap(), + config)); + try!(source.update()); + + let mut compilation = try!(ops::compile(manifest_path, &options.compile_opts)); + compilation.tests.sort(); + Ok(compilation) +} + +/// Run the unit and integration tests of a project. +fn run_unit_tests(options: &TestOptions, + test_args: &[String], + compilation: &Compilation) + -> CargoResult> { + let config = options.compile_opts.config; + let cwd = options.compile_opts.config.cwd(); + + let mut errors = Vec::new(); + + for &(_, ref exe) in &compilation.tests { + let to_display = match util::without_prefix(exe, &cwd) { + Some(path) => path, + None => &**exe, + }; + let mut cmd = try!(compilation.target_process(exe, &compilation.package)); + cmd.args(test_args); + try!(config.shell().concise(|shell| { + shell.status("Running", to_display.display().to_string()) + })); + try!(config.shell().verbose(|shell| { + shell.status("Running", cmd.to_string()) + })); + + if let Err(e) = ExecEngine::exec(&mut ProcessEngine, cmd) { + errors.push(e); + if !options.no_fail_fast { + break + } + } + } + Ok(errors) +} - let mut errors = match error { - None => Vec::new(), - Some(err) => vec![err], - }; +#[allow(deprecated)] // connect => join in 1.3 +fn run_doc_tests(options: &TestOptions, + test_args: &[String], + compilation: &Compilation) + -> CargoResult> { + let mut errors = Vec::new(); + let config = options.compile_opts.config; + let libs = compilation.package.targets().iter() + .filter(|t| t.doctested()) + .map(|t| (t.src_path(), t.name(), t.crate_name())); for (lib, name, crate_name) in libs { try!(config.shell().status("Doc-tests", name)); - let mut p = try!(compile.rustdoc_process(&compile.package)); + let mut p = try!(compilation.rustdoc_process(&compilation.package)); p.arg("--test").arg(lib) .arg("--crate-name").arg(&crate_name) - .cwd(compile.package.root()); + .cwd(compilation.package.root()); - for &rust_dep in &[&compile.deps_output, &compile.root_output] { + for &rust_dep in &[&compilation.deps_output, &compilation.root_output] { let mut arg = OsString::from("dependency="); arg.push(rust_dep); p.arg("-L").arg(arg); } - for native_dep in compile.native_dirs.values() { + for native_dep in compilation.native_dirs.values() { p.arg("-L").arg(native_dep); } @@ -60,11 +137,11 @@ pub fn run_tests(manifest_path: &Path, p.arg("--test-args").arg(&test_args.connect(" ")); } - for feat in compile.features.iter() { + for feat in compilation.features.iter() { p.arg("--cfg").arg(&format!("feature=\"{}\"", feat)); } - for (_, libs) in compile.libraries.iter() { + for (_, libs) in compilation.libraries.iter() { for &(ref target, ref lib) in libs.iter() { // Note that we can *only* doctest rlib outputs here. A // staticlib output cannot be linked by the compiler (it just @@ -91,81 +168,11 @@ pub fn run_tests(manifest_path: &Path, shell.status("Running", p.to_string()) })); if let Err(e) = ExecEngine::exec(&mut ProcessEngine, p) { - errors.push(Box::new(e)); - if !options.no_fail_fast { - break - } - } - } - if errors.is_empty() { - Ok(None) - } else if errors.len() == 1 { - Ok(Some(errors.pop().unwrap())) - } else { - let err = internal("Multiple tests failed"); - Ok(Some(err)) - } - -} - -pub fn run_benches(manifest_path: &Path, - options: &TestOptions, - args: &[String]) -> CargoResult>> { - let mut args = args.to_vec(); - args.push("--bench".to_string()); - - match try!(build_and_run(manifest_path, options, &args)).1 { - Some(err) => Ok(Some(err)), - None => Ok(None) - } -} - -// This function always has to return the Compilation, regardless of errors. -// Otherwise --no-fail-fast is not possible. -fn build_and_run<'a>(manifest_path: &Path, - options: &TestOptions<'a>, - test_args: &[String]) - -> CargoResult<(Compilation<'a>, Option>)> { - let config = options.compile_opts.config; - let mut source = try!(PathSource::for_path(&manifest_path.parent().unwrap(), - config)); - try!(source.update()); - - let mut compile = try!(ops::compile(manifest_path, &options.compile_opts)); - if options.no_run { return Ok((compile, None)) } - compile.tests.sort(); - - let cwd = config.cwd(); - let mut errors = Vec::new(); - for &(_, ref exe) in &compile.tests { - let to_display = match util::without_prefix(exe, &cwd) { - Some(path) => path, - None => &**exe, - }; - let mut cmd = try!(compile.target_process(exe, &compile.package)); - cmd.args(test_args); - try!(config.shell().concise(|shell| { - shell.status("Running", to_display.display().to_string()) - })); - try!(config.shell().verbose(|shell| { - shell.status("Running", cmd.to_string()) - })); - - if let Err(e) = ExecEngine::exec(&mut ProcessEngine, cmd) { errors.push(e); if !options.no_fail_fast { break } } } - if errors.is_empty() { - Ok((compile, None)) - } else if errors.len() == 1 { - // Just one error occured => we can return it. - Ok((compile, Some(Box::new(errors.pop().unwrap())))) - } else { - // Multiple tests failed => Create a more generic error - let err = internal("Multiple tests failed"); - Ok((compile, Some(err))) - } + Ok(errors) } diff --git a/src/cargo/util/errors.rs b/src/cargo/util/errors.rs index 4c24b34c5..622e09c1e 100644 --- a/src/cargo/util/errors.rs +++ b/src/cargo/util/errors.rs @@ -131,6 +131,50 @@ impl fmt::Debug for ProcessError { } } +// ============================================================================= +// Cargo test errors. + +/// Error when testcases fail +pub struct CargoTestError { + pub desc: String, + pub exit: Option, + cause: Option, +} + +impl fmt::Display for CargoTestError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(&self.desc, f) + } +} + +impl fmt::Debug for CargoTestError { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fmt::Display::fmt(self, f) + } +} + +impl Error for CargoTestError { + fn description(&self) -> &str { &self.desc } + #[allow(trivial_casts)] + fn cause(&self) -> Option<&Error> { + self.cause.as_ref().map(|s| s as &Error) + } +} + +#[allow(deprecated)] // connect => join in 1.3 +impl<'a> From<&'a [ProcessError]> for CargoTestError { + fn from(errors: &[ProcessError]) -> Self { + if errors.len() == 0 { panic!("Cannot create CargoTestError from empty Vec") } + let desc = errors.iter().map(|error| error.desc.clone()).collect::>().connect("\n"); + CargoTestError { + desc: desc, + exit: errors[0].exit, + cause: None, + } + } +} + + // ============================================================================= // Concrete errors @@ -274,6 +318,7 @@ impl CargoError for git2::Error {} impl CargoError for json::DecoderError {} impl CargoError for curl::ErrCode {} impl CargoError for ProcessError {} +impl CargoError for CargoTestError {} impl CargoError for CliError {} impl CargoError for toml::Error {} impl CargoError for toml::DecodeError {} diff --git a/src/cargo/util/mod.rs b/src/cargo/util/mod.rs index b47e8095d..95b01a723 100644 --- a/src/cargo/util/mod.rs +++ b/src/cargo/util/mod.rs @@ -2,7 +2,7 @@ pub use self::config::Config; pub use self::dependency_queue::Dependency; pub use self::dependency_queue::{DependencyQueue, Fresh, Dirty, Freshness}; pub use self::errors::{CargoResult, CargoError, ChainError, CliResult}; -pub use self::errors::{CliError, ProcessError}; +pub use self::errors::{CliError, ProcessError, CargoTestError}; pub use self::errors::{Human, caused_human}; pub use self::errors::{process_error, internal_error, internal, human}; pub use self::graph::Graph; -- 2.30.2